Skip to content

Fix machine details for a machine with tags#236

Merged
sparkiegeek merged 4 commits intocanonical:masterfrom
sparkiegeek:render-tags
Feb 28, 2020
Merged

Fix machine details for a machine with tags#236
sparkiegeek merged 4 commits intocanonical:masterfrom
sparkiegeek:render-tags

Conversation

@sparkiegeek
Copy link
Copy Markdown
Contributor

@sparkiegeek sparkiegeek commented Feb 28, 2020

Viscera layer produces a Tags.Managed object for machines with tags, this can't be simply str()ified.

Added a simple test for this behaviour, but there's a gap of testing for machine details in general.

Drive-by cleaned up a yaml.load

An example of how this looks for the plain renderer:

+---------------+-------------+
| Hostname      | name-wn44bg |
| Status        | READY       |
| Image         | (none)      |
| Power         | Off         |
| Power Type    | Manual      |
| Arch          | amd64       |
| #CPUs         | 2           |
| RAM           | 1.0 GB      |
| Interfaces    | 0 physical  |
| IP addresses  |             |
| Resource pool | pool1       |
| Zone          | zone1       |
| Owner         | (none)      |
| Tags          | tag1        |
|               | tag2        |
+---------------+-------------+

Viscera layer produces a Tags.Managed object for machines with tags,
this can't be str()ified.
@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 28, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@6822d83). Click here to learn what that means.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #236   +/-   ##
=========================================
  Coverage          ?   67.98%           
=========================================
  Files             ?       70           
  Lines             ?     5726           
  Branches          ?      988           
=========================================
  Hits              ?     3893           
  Misses            ?     1610           
  Partials          ?      223
Impacted Files Coverage Δ
maas/client/bones/testing/server.py 85.25% <100%> (ø)
maas/client/flesh/tables.py 46.74% <60%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6822d83...e250f29. Read the comment docs.

output = yaml.safe_load(
cmd.execute(origin, options, target=tabular.RenderTarget.yaml)
)
self.assertEqual(output.get("tags"), "tag1, tag2")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make sense for the yaml renderer to render the tags as a list of strings?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, fixed!

@bjornt
Copy link
Copy Markdown
Contributor

bjornt commented Feb 28, 2020

+1

@bjornt bjornt closed this Feb 28, 2020
@bjornt bjornt reopened this Feb 28, 2020
Copy link
Copy Markdown
Contributor

@bjornt bjornt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@sparkiegeek sparkiegeek merged commit 069e758 into canonical:master Feb 28, 2020
@sparkiegeek sparkiegeek deleted the render-tags branch February 28, 2020 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants